Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cli: parse out redundant "all:" revset modifier #3658

Merged
merged 2 commits into from
May 10, 2024
Merged

cli: parse out redundant "all:" revset modifier #3658

merged 2 commits into from
May 10, 2024

Conversation

yuja
Copy link
Contributor

@yuja yuja commented May 10, 2024

#3654

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@yuja
cli: use command helper to check if path argument is parsable as revset
b885a76 
There should be no reason to use low-level API.
ilyagr
ilyagr reviewed May 10, 2024
View reviewed changes
Copy link
Contributor

@ilyagr ilyagr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

cli/src/commands/log.rs Outdated Show resolved Hide resolved
cli/tests/test_revset_output.rs Show resolved Hide resolved
@yuja
cli: parse out redundant "all:" revset modifier in arguments and temp…
1d62761 
…lates

This also means "all:" is allowed in default revsets (such as "revsets.log"),
but that seems okay. In revset aliases, "all:" isn't allowed because aliases
may be expanded to sub-expression position.

Closes jj-vcs#3654
@yuja yuja force-pushed the push-pxnonulrmnrt branch from b1362cf to 1d62761 Compare May 10, 2024 06:19
@yuja yuja enabled auto-merge (rebase) May 10, 2024 06:26
@yuja yuja merged commit 3307696 into jj-vcs:main May 10, 2024
16 checks passed
@yuja yuja deleted the push-pxnonulrmnrt branch May 10, 2024 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilyagr ilyagr ilyagr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yuja @ilyagr